Use prepended modules instead of undef for OS-specific code
#18305
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dduugg
commented
Sep 11, 2024
dduugg
commented
Sep 11, 2024
MikeMcQuaid
reviewed
Sep 11, 2024
Yes, that would be very nice, but right now sorbet/sorbet#5025 requires the |
dduugg
commented
Sep 13, 2024
dduugg
force-pushed
the
no-undefs
branch
6 times, most recently
from
466fdf6 to
4465375
Compare
MikeMcQuaid
approved these changes
Sep 19, 2024
There was a problem hiding this comment.
Thanks @dduugg! Still have some thoughts about the module naming/hierarchy but both the discussions and changes can 100% be done post-merge so no feedback here is blocking. Feel free to self-merge whenever you feel happy with it. Great work!
carlocab
approved these changes
Sep 19, 2024
6 tasks
|
This seems to be causing an error at Homebrew/homebrew-core#191090 |
29 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
brew stylewith your changes locally?brew typecheckwith your changes locally?brew testswith your changes locally?Relates to #17998
Switches from using
undef, which results in (suppressed) sorbet errors, with prepended modules.The latest proposal on the thread suggested creating an interface with three implementations, rather than using the generic-OS version in the default implementation, with methods undefined in OS-specific code. In building that out, I noticed that there wasn't a consistent, common interface among the OS variations, specifically in the case of
Formula.Thus, I've landed on OS extensions written as modules, which are then prepended to be used ahead of the generic implementations. (Note that in some cases, the modules are prepended to the singleton class, which admittedly gets pretty esoteric.)